feat: attach image drives to warm starts - #398

This diagnostic is emitted for image mounts too, so it incorrectly calls the offending path a "volume mount path". That makes CLI errors misleading when users pass --image-mount or --image-mount-rw; use a generic mount-path message (or pass the mount kind into the validator).

The requested size_mb is used to create the volume but is not retained in ImageMount. Consequently, image_mounts_model cannot report it and any API/lifecycle path that reconstructs an image mount from this metadata loses the requested writable-view size (and defaults it on recreation). Preserve the size in the internal mount metadata, or derive and serialize it from the associated volume consistently.

This rollback path discards deletion errors. If catalog deletion fails (for example due to a transient repository/storage error), the newly created sandbox-scoped volume remains in the catalog and its backing data is orphaned, while the request still returns the original failure. At minimum, surface/log the cleanup failure with the volume ID, and provide a retry/GC path for volumes that could not be rolled back.

HashMap iteration makes the imageMounts array order nondeterministic across otherwise identical responses. Sort the paths (as the volume resolver does) before collecting so clients, snapshot tests, and serialized output remain stable.

The duplicate check here compares the image mount's already-normalized key with the explicit volume mount keys as supplied by the client. Explicit keys are only normalized later in resolve_volume_mounts, where the normalized map can overwrite an earlier entry (and both drives have already been prepared). Thus equivalent paths such as /mnt/models/ and /mnt/models, or overlapping paths such as /mnt and /mnt/models, can bypass this check and leave two attached drives while metadata contains only one path. Normalize all mount paths and reject collisions/overlaps before combining the maps.

Suggestion:

Filtering sandbox_scoped records after list_page makes hidden records consume the requested page size and continuation cursor. If a page contains only image-mount volumes, this returns an empty page with a cursor already advanced past those records; if hidden records precede visible ones, clients can receive short pages and pagination can skip visible volumes. The catalog query needs to exclude scoped records before applying the limit/cursor, or the API must keep fetching pages until it has filled the requested visible page and return a cursor based on the last scanned record.

Filtering scoped records after list_page has already applied the limit and cursor can permanently omit visible volumes. For example, if a page contains only scoped records followed by persistent records, next_token is based on the end of that page and the persistent records are skipped; similarly, a full page of scoped records may return an empty result with no usable continuation. Apply the visibility filter while constructing the paginated catalog result (and derive the cursor from returned visible records), or fetch/advance pages until the requested visible page is filled.

A transient delete failure is only logged and then ignored, while delete_sandbox_impl proceeds to remove the sandbox metadata (and terminal cleanup has no later metadata-driven retry). This can permanently orphan a sandbox-scoped volume after a repository/filesystem failure. Propagate a non-NotFound error from this helper so sandbox deletion remains retryable, or persist a cleanup task/record that will retry the volume deletion.

This deletes based solely on the metadata path classification (image_mounts contains the mount path), without verifying that the volume record is actually sandbox_scoped. A stale/corrupt record, or a metadata update that pairs an image path with an existing persistent volume ID, can therefore permanently delete a user-managed volume during sandbox deletion. Fetch the record and only delete when sandbox_scoped is true (and otherwise report/retain the volume).

Suggestion:

This assumes path membership is sufficient to identify the lifecycle of a volume. If persisted metadata is incomplete (for example, an image_mounts entry has no corresponding volume_mounts entry), the referenced scoped volume is never returned and cannot be released/deleted; if the path and ID are mismatched, a persistent volume is classified as scoped. Cleanup should validate the volume record's sandbox_scoped flag and handle inconsistent metadata explicitly rather than relying only on the two maps.

This adds a required parameter to the public Client::create_sandbox/create_cold_sandbox methods, so any downstream users of the aenv client crate now fail to compile even when they do not use image mounts. Please preserve source compatibility (for example with a separate image-mount-aware method or an options/request type) or account for this as a breaking API change.

For invalid image mount paths, normalize_mount_path still emits diagnostics such as volume mount path must be... and volume mount path conflicts.... Because this function is now used by the image-mount flags, these messages identify the wrong input type and make CLI errors misleading; pass a mount-kind-specific label into normalization or map the errors to image-mount wording here.

If this future is cancelled after one or more create_from_snapshot calls complete (for example, while resolving the next image), the Err cleanup below is never reached because cancellation drops the future. The created IDs are only held in this stack frame, so those sandbox-scoped volumes become orphaned. Use a drop/cancellation guard or an operation that owns and rolls back the partial set on cancellation; the same issue applies to snapshot restoration below.

Iterating this HashMap directly makes the order of imageMounts vary between responses for the same sandbox. The existing volume-mount response is also map-backed, but this new field has no ordering guarantee or test; sort by guest path before collecting if clients/tests rely on stable API output.

The requested sizeMB is discarded when the sandbox metadata is built: ImageMount only retains image and read_only, and this response always emits size_mb: None. Consequently, a client that GETs a sandbox and reuses its imageMounts loses any custom mount size and silently recreates it at the default 64 GiB. Preserve the size in the internal metadata (and snapshot/fork metadata as appropriate) or populate this field from the mounted volume record.

Suggestion:

This insertion silently overwrites an earlier normalized path from the snapshot. The earlier child volume has already been created but is no longer present in either returned map, so it is neither cleaned up on later failure nor represented in the sandbox. Reject duplicate/overlapping normalized snapshot paths before creating the child (or clean up the overwritten ID).

Filtering after fetching a backend page breaks the continuation contract: if this page contains only sandbox-scoped records, the response is empty while x_next_token advances past them. Clients that stop on an empty page (and clients expecting a full page) can miss persistent volumes later in the listing. Apply the scope filter before pagination, or fetch subsequent backend pages until the response is filled/exhausted and return the corresponding token.

This excludes image-mount volumes from the orchestrator's freeze/snapshot/publish step even when an image mount is writable (read_only == false). The API later serializes their snapshot_volume_state, but without freezing/restacking/publishing the current upper layer, the committed snapshot can contain only the original image layers and lose writes made to the image mount on capture. Writable scoped mounts need to participate in capture (while still being deleted after capture), or must be explicitly rejected as non-snapshotable.

Deletion errors are only logged here, after which delete_sandbox_impl proceeds to remove_deleted_sandbox. A transient catalog/storage failure therefore leaves the sandbox-scoped volume orphaned with no sandbox metadata left for the normal delete retry path to discover. Propagate a non-NotFound error (or persist a retryable cleanup record) so sandbox deletion remains incomplete and retryable until these volumes are removed.

Suggestion:

This classifies a volume as deletable solely from the mount path and never verifies that the referenced VolumeRecord is actually sandbox_scoped. If a persisted/legacy record has an image-mount entry pointing at a persistent volume (or the same volume ID is also mounted at a normal path), terminal cleanup will release it and delete_scoped_volumes will delete user data. The cleanup classification should be based on the volume's scoped ownership/invariant, and inconsistent path/ID mappings should be rejected rather than treated as safe to delete.

Suggestion:

NewSandbox and NewColdSandbox are public structs with public fields, so adding a required field makes existing downstream struct literals fail to compile, independently of the Client method signature change. Consider adding the field with a builder/options API or otherwise preserving construction compatibility for this public request type.

This is a public client API, so adding a required positional parameter is a source-incompatible change for any downstream caller of Client::create_sandbox (and likewise for create_cold_sandbox). Consider preserving the existing methods and adding *_with_image_mounts/an options struct, or otherwise provide a compatibility path for clients that do not use image mounts.

Suggestion:

For invalid image-mount paths, normalize_mount_path emits diagnostics such as “volume mount path must...” and “volume mount path contains...”. This makes the new CLI flag report the wrong resource type; pass an image-specific path-validation error (or parameterize the helper) so users can distinguish image-mount failures from --volume failures.

Suggestion:

This cleanup runs only when the inner future returns Err. If the request is cancelled while a later image is being resolved/created, the future is dropped with already-created volume IDs still in created, so no deletion runs and sandbox-scoped volumes can be orphaned. Use a cancellation-safe cleanup guard (and apply the same pattern to restore_snapshot_volume_mounts, whose local volume_ids/image_mounts are also cleaned only on explicit error paths).

Suggestion:

The scoped IDs are cleaned only when prepare_volume_mounts_inner returns an error. If this future is cancelled while reserving/resolving drives, the already-created image-mount volumes are dropped with no cleanup guard and remain orphaned. This should use cancellation-safe ownership/cleanup, not only an is_err() branch.

Suggestion:

Filtering after list_page has already applied the limit makes pagination depend on hidden records. A page can contain only sandbox-scoped volumes and return an empty list with a non-empty cursor, and callers that treat an empty page as the end can stop before seeing later team volumes. Fetch/filter until the requested number of visible records is filled, or move the visibility predicate into the catalog query while computing the cursor from visible records.

Suggestion:

This permanently deletes every ID classified as scoped solely from the mount-path metadata. The cleanup has already cleared ownership, so a stale/corrupt record (or an ID aliased by a regular mount) can cause a persistent volume to be deleted as an image-mount volume. Before deletion, load each VolumeRecord and require sandbox_scoped (and ensure it is not referenced by any persistent mount), or otherwise reject/repair inconsistent metadata rather than trusting the path classification.

Suggestion:

Deletion errors are only logged and the caller still marks the sandbox Done/removes its metadata. A transient repository or filesystem failure therefore leaves the sandbox-scoped volume orphaned with no retry path, unlike the surrounding cleanup operations that return an error and retain deletion progress. Propagate a deletion failure (or persist a cleanup task) so a subsequent delete retries it before completing.

Suggestion:

This split only iterates volume_mounts, so an image_mounts entry without a matching backing-volume entry is silently omitted from cleanup and from snapshot handling. The API normally creates both maps together, but metadata is persisted/deserialized and can be partially written or migrated; cleanup should validate the one-to-one invariant (or return/report an inconsistency) instead of treating it as a valid empty set, so orphaned scoped volumes are not hidden permanently.

Suggestion:

The index is taken before filtering, so a post-boot read-only volume in the middle leaves a device-letter hole. For example, [writable, read-only volume, writable] emits vdc for the first and vde for the second, while the drive list/device attachment remains contiguous (vdc, vdd); init will therefore mount the wrong device or fail. Filter first and then enumerate the remaining drives, while retaining the original index only for the post-boot mount list.

Suggestion:

These public client methods now require a new positional argument, so any downstream caller (and any command/test path outside this repository) that uses the existing API will fail to compile. Prefer preserving the existing methods and adding image-mount-aware variants, or make the new parameter optional via a request/options type, while updating internal callers.

This cleanup only runs when the future resolves to Err. If the request is cancelled while an image is being resolved or created (for example, after earlier mounts have already been created), the async function is dropped and created.volume_ids() is never deleted. The partially created sandbox-scoped volumes can therefore leak indefinitely because no sandbox owns them yet; use a cancellation-safe guard/cleanup mechanism around the in-progress creation.

Only exact duplicates are rejected here. Two requested image mounts such as /mnt/data and /mnt/data/sub both pass, and prepare_volume_mounts_inner only checks exact key collisions before delegating to resolve_volume_mounts; although that resolver rejects overlaps among the combined map, the API's image paths are normalized and it does catch there actually, but this validation is not performed until after all image volumes have been created. A malformed overlapping request can therefore create all sandbox-scoped volumes and only then fail, relying on cleanup; validate ancestor/descendant overlap while processing requests so the request is rejected before unnecessary volume creation.